Implement scroll up and down #103

zblz · 2017-11-21T17:21:48Z

This PR implements handling of the SU and SD CSI commands in pyte.Stream and the corresponding scroll_up and scroll_down methods in pyte.Screen.

…lar dict

superbobry · 2017-12-09T23:19:30Z

Thanks for the contribution! Could you point me to documentation for these escape sequences?

zblz · 2017-12-10T20:51:54Z

They are (sparsely) documented here: http://invisible-island.net/xterm/ctlseqs/ctlseqs.html

The most common use of this is when you call clear inside of a tmux session with a status bar. Instead of clearin everything and redrawing the status bar, tmux will scroll the non-status-bar lines out of the screen using scroll up. We also looked at vttest requirements to check its behaviour.

mvilim · 2019-02-07T03:14:53Z

Any chance this can be merged?

I ran into the lack of scroll support when using the Python curses module. Scrolling can be invoked implicitly in ncurses (I believe this is due to hardscroll.c, which appears to detect 'scroll-like' updates, even if a scroll was not explicitly requested.)

Here is a script that demonstrates psuedo-scroll behavior. With TERM set to a terminal with scrolling capability (e.g. xterm-256color), this script fails to update the 0, 0 cell to 1 (i.e. the first assert should fail) because ncurses decides to send a scroll command. With TERM set to linux the script passes.

import curses
import pyte
import os


def psuedo_scroll(stdscr):
    stdscr.addstr(0, 0, '0')
    stdscr.addstr(2, 0, '1')
    stdscr.refresh()
    stdscr.addstr(0, 0, '1')
    stdscr.addstr(2, 0, '2')
    stdscr.refresh()


pid, fd = os.forkpty()
if pid == 0:
    curses.wrapper(psuedo_scroll)
else:
    screen = pyte.Screen(80, 24)
    stream = pyte.ByteStream(screen)
    # pass through bytes directly
    stream.select_other_charset('@')
    while True:
        try:
            output = os.read(fd, 1024)
            stream.feed(output)
        except OSError:
            break
    assert(screen.buffer[0][0].data == '1')
    assert(screen.buffer[2][0].data == '2')

I don't know of any easy way to set the term capabilities with greater granularlity than TERM (to disable scrolling but leave support for other features (e.g. color)), so I would prefer to not use TERM=linux. Additionally, merging this would save some time for others who run into the issue (it took some debugging to trace observed incorrect terminal state to the scroll behavior).

I tested this pull request and it solves both my original issue and causes the above test to pass.

altoidbox · 2020-07-08T14:25:12Z

pyte/screens.py

+                self.buffer[y] = self.buffer[y + lines].copy()
+            else:
+                self.buffer[y].clear()
+        self.dirty = set(lines_to_scroll)


Should update the dirty set rather than replace it.

Suggested change

self.dirty = set(lines_to_scroll)

self.dirty.update(lines_to_scroll)

altoidbox · 2020-07-08T14:28:12Z

pyte/screens.py

@@ -1043,6 +1048,26 @@ def debug(self, *args, **kwargs):
        By default is a noop.
        """

+    def scroll_up(self, lines):
+        """Scroll up `lines` lines."""
+        lines_to_scroll = range(self.margins.top, self.margins.bottom + 1)


We should handle when margins is None.

Suggested change

lines_to_scroll = range(self.margins.top, self.margins.bottom + 1)

margins = self.margins or Margins(0, self.lines - 1)

lines_to_scroll = range(margins.top, margins.bottom + 1)

altoidbox · 2020-07-08T14:54:13Z

pyte/screens.py

+            if y + lines in set(lines_to_scroll):
+                self.buffer[y] = self.buffer[y + lines].copy()
+            else:
+                self.buffer[y].clear()


Scrolling is similar to indexing multiple times, is it not? If we follow its example (and it may be more efficient) we can simply move the lines we want to move and pop the lines we want to erase, rather than copying and clearing the dictionaries. We can also improve efficiency by checking the bottom margin rather than creating a set to test for set inclusion.

Suggested change

if y + lines in set(lines_to_scroll):

self.buffer[y] = self.buffer[y + lines].copy()

else:

self.buffer[y].clear()

if y + lines <= margins.bottom:

self.buffer[y] = self.buffer[y + lines]

else:

self.buffer.pop(y, None)

altoidbox · 2020-07-08T16:55:51Z

pyte/escape.py

+SU = "S"
+
+#: *Scroll down*
+SD = "T"


While the xterm documentation states that 'T' is the correct code for Scroll Down (SD), the xterm code is implemented to handle the incorrect code '^' and treats 'T' as either mouse tracking or SD (depending on the arguments).
I'm not sure how many (or if any) applications still use the incorrect code.

altoidbox · 2020-07-08T17:10:32Z

pyte/screens.py

+    def scroll_up(self, lines):
+        """Scroll up `lines` lines."""


The documentation states that a scroll with no parameters scrolls 1 line.

Suggested change

def scroll_up(self, lines):

"""Scroll up `lines` lines."""

def scroll_up(self, lines=None):

"""Scroll up `lines` lines.

:param lines: number of lines to scroll up. defaults to 1 if unspecified.

"""

lines = lines or 1

altoidbox · 2020-07-08T17:29:25Z

pyte/screens.py

+    def scroll_down(self, lines):
+        """Scroll down `lines` lines."""
+        lines_to_scroll = range(self.margins.bottom, self.margins.top - 1, -1)
+        for y in lines_to_scroll:
+            if y - lines in set(lines_to_scroll):
+                self.buffer[y] = self.buffer[y - lines].copy()
+            else:
+                self.buffer[y].clear()
+        self.dirty = set(lines_to_scroll)


Similar to scroll_up, this should also default to 1 line if the argument is unspecified. However, an argument of 0 is used to reset mouse tracking (may be xterm specific). In any case, if we get an argument of 0, we don't want to scroll at all. And if we don't get an argument we do want to scroll exactly 1 line. However, looking at the current implementation of _parser_fsm, we may be unable to differentiate between receiving no arguments and receiving a 0 as the first argument. Reading vt100.net, they seem to indicate that for their purposes there is no need to distinguish between the value 0 and an unspecified argument (i.e., an explicit 0 still implies the default). Perhaps this is an xterm specific issue in this case.

Any other changes made to scroll_up should be appropriately mirrored here in scroll_down.

srstevenson and others added 5 commits November 21, 2017 17:11

Add SU and SD constants to pyte.escape

4a2dcb2

Add scroll_down and scroll_up entries to csi dict

1da5b4a

Override copy method so that StaticDefaultDict does not return a regu…

f15ea30

…lar dict

Implement Screen.scroll_down and Screen.scroll_up

97843a0

Test Screen.scroll_down and Screen.scroll_up

2e86d5d

altoidbox reviewed Jul 8, 2020

View reviewed changes

superbobry force-pushed the master branch 3 times, most recently from f8e191c to fa5255a Compare October 11, 2021 16:20

superbobry force-pushed the master branch 2 times, most recently from 6cea8ec to 259ee02 Compare November 12, 2023 11:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement scroll up and down #103

Implement scroll up and down #103

zblz commented Nov 21, 2017

superbobry commented Dec 9, 2017

zblz commented Dec 10, 2017

mvilim commented Feb 7, 2019

altoidbox Jul 8, 2020

altoidbox Jul 8, 2020

altoidbox Jul 8, 2020

altoidbox Jul 8, 2020

altoidbox Jul 8, 2020 •

edited

Loading

altoidbox Jul 8, 2020 •

edited

Loading

	self.dirty = set(lines_to_scroll)
	self.dirty.update(lines_to_scroll)

	lines_to_scroll = range(self.margins.top, self.margins.bottom + 1)
	margins = self.margins or Margins(0, self.lines - 1)
	lines_to_scroll = range(margins.top, margins.bottom + 1)

Implement scroll up and down #103

Are you sure you want to change the base?

Implement scroll up and down #103

Conversation

zblz commented Nov 21, 2017

superbobry commented Dec 9, 2017

zblz commented Dec 10, 2017

mvilim commented Feb 7, 2019

altoidbox Jul 8, 2020

Choose a reason for hiding this comment

altoidbox Jul 8, 2020

Choose a reason for hiding this comment

altoidbox Jul 8, 2020

Choose a reason for hiding this comment

altoidbox Jul 8, 2020

Choose a reason for hiding this comment

altoidbox Jul 8, 2020 • edited Loading

Choose a reason for hiding this comment

altoidbox Jul 8, 2020 • edited Loading

Choose a reason for hiding this comment

altoidbox Jul 8, 2020 •

edited

Loading

altoidbox Jul 8, 2020 •

edited

Loading